🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/20052

• 📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

⚠️ 16 Awaiting Approval

As of commit ee0b023 with merge base 2759ef1 ():

This comment was automatically generated by Dr. CI and updates every 15 minutes.

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@claude review

Claude finished @psiddh's task in 2m 28s —— View job

Code Review: Qualcomm Windows native build

• Read the CMake changes
• Read the build.ps1 script
• Read the header / source changes
• Post review feedback

Overall this is a clean, well-scoped change. The MSVC/CMAKE_HOST_WIN32 guards are applied consistently and the x86_64|AMD64 and CMAKE_HOST_WIN32 fixes are correct (the host always produces a runnable flatc.exe/flatcc.exe even when cross-compiling for ARM64). A few things worth confirming before merge.

1. setup.py — dropped %CMAKE_CACHE_DIR%/ prefix may break artifact resolution (please confirm)

BuiltExtension(
    src_dir="backends/qualcomm/%BUILD_TYPE%/",   # was: %CMAKE_CACHE_DIR%/backends/qualcomm/%BUILD_TYPE%/
    src=("PyQnnManagerAdaptor*.pyd" if _is_windows() else "PyQnnManagerAdaptor.*"),
    ...
)

Removing the %CMAKE_CACHE_DIR%/ placeholder changes BuiltExtension._get_build_dir() to return Path(".") (repo root) instead of the cmake-out cache dir (setup.py:362-374). Two consequences:

• The .pyd is now resolved relative to the repo root (./backends/qualcomm/Release/...), not the build cache. Is that actually where the build emits it during install_executorch.bat? It also disables the is_cmake_artifact_used() gating, since ./CMakeCache.txt won't exist (setup.py:376-380).
• The sibling qnn_executorch_backend BuiltFile directly above (setup.py:1116) still uses %CMAKE_CACHE_DIR%/backends/qualcomm/%BUILD_TYPE%/. The two are now inconsistent — if the placeholder removal is correct for the .pyd, why not for the .dll?

Please confirm this is intentional and not a path that only happens to work in your local layout.

2. backends/qualcomm/CMakeLists.txt:285-293 — WINDOWS_EXPORT_ALL_SYMBOLS + explicit __declspec(dllexport) are redundant

target_compile_definitions(qnn_executorch_backend PRIVATE QNN_EXECUTORCH_BUILDING_DLL)
set_target_properties(qnn_executorch_backend PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS ON)

QNN_EXECUTORCH_BUILDING_DLL switches QNN_EXECUTORCH_EXPORT to __declspec(dllexport) (runtime/QnnExecuTorch.h:30-38), and WINDOWS_EXPORT_ALL_SYMBOLS auto-exports everything. You generally want one or the other — combining them is redundant and auto-export will pull the full symbol table into the export table. If the explicit __declspec annotations on the C API are sufficient, consider dropping WINDOWS_EXPORT_ALL_SYMBOLS; if you rely on auto-export, the macro/define is unnecessary. Not a bug, just worth tightening.

3. MSVC loses dead-code stripping vs. the GCC/Clang path

The non-MSVC release path uses -ffunction-sections -fdata-sections + LINKER:-s,--gc-sections to strip unused code (binary size matters for ET). The MSVC branch only sets /O2. For parity consider /Gy (+ /Gw) and /OPT:REF,ICF at link time. Optional, but Windows binaries will otherwise be noticeably larger.

4. MSVC builds are no longer -Werror

add_compile_options("/wd4996") replaces -Wall -Werror on MSVC, so Windows warnings won't fail the build. Reasonable for bring-up, but flagging that MSVC is now less strict than the other toolchains.

5. runner.h / multimodal_runner.h — enum EvalMode moved private → public

This isn't obviously Windows-related (it's an API surface change, not a compile fix). If it's required because something references Runner::EvalMode externally on MSVC, a one-line note in the commit would help; otherwise it looks unrelated to this PR's scope.

Minor / nits

• build.ps1 — the ARM64/Hexagon blocks pass the source dir positionally ($PrjRoot ... -B$BuildRoot) while the x86 block uses -S$PrjRoot -B$BuildRoot. Both work; aligning them reduces confusion.
• build.ps1 — the header comment says ARM64 toolchain (clang-cl/lld-link) is "validated lazily inside the build-arm64-windows block," but I don't see that validation in the block. Either add it or drop the comment.
• build.ps1:~270 — git submodule update --init for extension/llm/tokenizers runs unconditionally even though the comment says tokenizers is intentionally skipped on Windows. It's non-fatal, but the comment and the action read as contradictory.
• aot/wrappers/CMakeLists.txt — moving TensorWrapper.cpp from PUBLIC to PRIVATE is a genuine correctness cleanup (.cpp sources shouldn't be propagated to consumers). 👍

Nothing here is blocking on its own, but item #1 should be confirmed since it affects the pip-install flow, not just the new Windows script.
• branch dev1/zhaoxul/support_windows_native_build

There are lint issues. @zhaoxul-qti Can you fix them so that we can merge this PR